Avoid logging the DC API key value - #713
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates simple/util/dc_client.py to prevent logging the actual DC API key when debugging is enabled, replacing it with a placeholder indicating whether the key is set or not. There are no review comments, so I have no feedback to provide.
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 0 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
The debug block logs the key itself, so the credential ends up in plain text in whatever log sink the deployment uses. On Cloud Run that means Cloud Logging, where it is readable by anyone holding roles/logging.viewer on the project — a considerably wider audience than those granted access to the secret. _DEBUG is hardcoded to True, so this happens on every run of the preprocessing job in any deployment, not just when debugging locally. Log whether the key is set instead of its value, which keeps the diagnostic signal (is it configured?) without disclosing the credential.
d7547f3 to
525fdf3
Compare
3 participants
What
simple/util/dc_client.pylogs the value of the Data Commons API key:This change logs whether the key is set rather than what it is, keeping the
diagnostic signal without writing the credential to the log sink.
Why it matters beyond local debugging
_DEBUGis hardcoded toTrue(line 38), not driven by an environment variable, sothis runs on every invocation in every deployment — not only when someone is debugging
locally.
In a Cloud Run deployment the line lands in Cloud Logging, where it is readable by
anyone with
roles/logging.vieweron the project. That is a much broader audience thanthe principals granted
roles/secretmanager.secretAccessoron the secret the key ismounted from, so the key effectively escapes the access model it was set up with. Log
sinks and exports widen it further.
We hit this on a Data Commons Platform deployment: the key appears in full in the
preprocessing job logs on every ingestion run.
Note on disclosure
We would have raised this privately, but this repository has no
SECURITY.mdandprivate vulnerability reporting is not enabled, so there was no non-public channel to
use. Opening a separate issue to suggest adding one.